Skip to content

fix(agentrun): unwrap data envelopes#1920

Open
guslegend0510 wants to merge 5 commits into
agentscope-ai:mainfrom
guslegend0510:fix/issue-1907-agentrun-data-envelope
Open

fix(agentrun): unwrap data envelopes#1920
guslegend0510 wants to merge 5 commits into
agentscope-ai:mainfrom
guslegend0510:fix/issue-1907-agentrun-data-envelope

Conversation

@guslegend0510

@guslegend0510 guslegend0510 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

AgentScope-Java Version

2.0.0-SNAPSHOT

Description

This PR fixes AgentRun data-plane JSON handling when successful responses are wrapped in a data envelope.

Background:

  • AgentRunDataPlaneHttp previously parsed response JSON from the root node.
  • AgentRun create/get responses return payloads under data, so fields like id and status were not visible to the client code.

Changes made:

  • Added unwrapData(...) in AgentRunDataPlaneHttp and applied it in getJson() and postJson().
  • Updated AgentRunDataPlaneHttpTest to mock wrapped responses and verify parsing still works.

Related issues:

How to test:

  • mvn -pl agentscope-extensions/agentscope-extensions-sandbox/agentscope-extensions-sandbox-agentrun -am -Dtest=AgentRunDataPlaneHttpTest test

@guslegend0510 guslegend0510 requested a review from a team June 25, 2026 15:05
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...nsions/sandbox/agentrun/AgentRunDataPlaneHttp.java 60.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@itxaiohanglover

itxaiohanglover commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Nice fix — the unwrapData helper is clean and the null/hasNonNull guard is good. One thing to keep an eye on: if any legitimate response body has a top-level data field that isn't an envelope, this would strip it. The test covering responses without the envelope is a nice touch for backward compat. Looks good overall.

@itxaiohanglover

itxaiohanglover commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Clean fix! Using hasNonNull("data") is more concise than the issue suggestion. Applying unwrap in both getJson and postJson covers all entry points. Test with both wrapped and unwrapped responses is good for backward compatibility.

@AgentScopeJavaBot AgentScopeJavaBot added bug Something isn't working area/ext/integration External protocols & middleware integrations labels Jul 11, 2026
AgentScopeJavaBot
AgentScopeJavaBot reviewed Jul 12, 2026
View reviewed changes

@AgentScopeJavaBot AgentScopeJavaBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI Review

Fixes AgentRun data-plane JSON parsing for data envelope responses. Clean minimal change. unwrapData may incorrectly strip legitimate data fields from non-envelope responses - consider structural guards.

Comment thread ...-agentrun/src/main/java/io/agentscope/extensions/sandbox/agentrun/AgentRunDataPlaneHttp.java
@@ -102,6 +102,13 @@ JsonNode getSandbox(String sandboxId) throws IOException {
return AgentRunRetry.withRetries(opt.getMaxRetries(), () -> getJson(url));
}

@AgentScopeJavaBot AgentScopeJavaBot Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] unwrapData checks only hasNonNull(data) without distinguishing envelope from regular response. Add structural guards (check outer node lacks id/status fields before unwrapping).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@AgentScopeJavaBot AgentScopeJavaBot AgentScopeJavaBot left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

area/ext/integration External protocols & middleware integrations bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[bug] AgentRunDataPlaneHttp 响应 JSON data 信封未解包

4 participants

@guslegend0510 @itxaiohanglover @AgentScopeJavaBot @guslegend